Skip to content

fix(dbcore): NumericQuery range crashes on a null value#6849

Open
chuenchen309 wants to merge 3 commits into
beetbox:masterfrom
chuenchen309:fix/numericquery-null-range-match
Open

fix(dbcore): NumericQuery range crashes on a null value#6849
chuenchen309 wants to merge 3 commits into
beetbox:masterfrom
chuenchen309:fix/numericquery-null-range-match

Conversation

@chuenchen309

Copy link
Copy Markdown

A range query against a nullable numeric field raises instead of returning False:

NumericQuery("rg_track_gain", "-10..0").match(Item())
TypeError: '<' not supported between instances of 'NoneType' and 'int'

NumericQuery.match() compares the value against rangemin/rangemax with no None guard. Nullable types (NullFloat, NullInteger) use None as their null value — unlike Integer/Float, whose null is 0, which is why year queries never hit this and the bug stayed hidden. Affected fields: album_id (None for every singleton) and the whole rg_* / r128_* family (None until ReplayGain has been run, i.e. by default). Point queries (rg_track_gain:5) were already fine — only the range branches.

What the correct behaviour is

The query's own col_clause() defines it: rg_track_gain >= ? AND rg_track_gain <= ? against NULL is never true in SQL, so the row is quietly excluded. match() should agree — this is the classic SQL-path/match-path divergence, and test_fast_vs_slow in the test file exists precisely to pin that contract (it just never covered a nullable field).

Reachability

Two callers, and the first has no SQL path at all:

  • library/models.py:1203Item.destination() evaluates paths: config rules through match() unconditionally. A rule like rg_track_gain:0.. crashes on any item lacking ReplayGain. Verified end-to-end:
    item.destination(path_formats=[("rg_track_gain:0..", "loud/$title"), ("default", "$title")])
    # TypeError
  • dbcore/db.py:805 — the Results slow path, used whenever clause() returns None (any flexattr subquery forces this). beet ls "rg_track_gain:0.. someflex:x" raises, while the same data through a fast SQL-only query returns [] cleanly.

Precedent

This is the sibling of #3461. Commit 2293d2f6 ("Fix crash when sorting by a nullable field missing on some items") fixed the identical None-comparison crash on the sort side — sort.py deliberately groups nulls together "matching SQLite's default ordering of NULLs", and its comment names NullInteger/NullFloat explicitly. The query side was missed.

Tests

Three cases (-10..0, 0.., ..0) asserting a range never matches a null value. All fail on master with the TypeError and pass here. No existing test fed a nullable field to NumericQuerytest/dbcore/test_query.py only uses year.

  • pytest test/dbcore/test_query.py — 146 passed (143 baseline + 3 new)
  • pytest test/test_library.py test/dbcore/ — 518 passed, no regressions
  • ruff check / ruff format --check / mypy clean; changelog checked with docstrfmt, sphinx-lint, and the unreleased-section check.

AI disclosure: drafted with Claude Code (Opus 4.8), including the root-cause trace and the tests. I ran the repro and the test suite locally and reviewed the diff before opening.

chuenchen309 and others added 2 commits July 16, 2026 23:53
NumericQuery.match() compares the field value against rangemin/rangemax
without a None guard, so a range query against a nullable numeric field
raises instead of returning False:

    NumericQuery("rg_track_gain", "-10..0").match(Item())
    TypeError: '<' not supported between instances of 'NoneType' and 'int'

Nullable types (NullFloat, NullInteger) use None as their null value, unlike
Integer/Float whose null is 0 -- which is why year queries never hit this.
Affected fields include album_id (None for every singleton) and the whole
rg_*/r128_* family (None until ReplayGain has been run, i.e. by default).

The query's own col_clause() defines the correct answer: `rg_track_gain >= ?
AND rg_track_gain <= ?` against NULL is never true in SQL, so the row is
quietly excluded. match() should agree, and now does. Point queries were
already fine.

Reachable from two callers, and the first has no SQL path at all:

- library/models.py:1203 -- Item.destination() evaluates `paths:` config
  rules through match() unconditionally. A rule like `rg_track_gain:0..`
  crashes on any item without ReplayGain.
- dbcore/db.py:805 -- the Results slow path, used whenever clause() returns
  None (any flexattr subquery forces this).

This is the sibling of beetbox#3461: 2293d2f fixed the identical None-comparison
crash on the sort side (sort.py deliberately groups nulls to match SQLite's
NULL ordering); the query side was missed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@chuenchen309
chuenchen309 requested a review from a team as a code owner July 16, 2026 15:54
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.63%. Comparing base (86f0f0a) to head (e4fa799).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6849   +/-   ##
=======================================
  Coverage   75.63%   75.63%           
=======================================
  Files         163      163           
  Lines       21312    21314    +2     
  Branches     3360     3361    +1     
=======================================
+ Hits        16119    16121    +2     
  Misses       4401     4401           
  Partials      792      792           
Files with missing lines Coverage Δ
beets/dbcore/query.py 90.71% <100.00%> (+0.04%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant